-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fix CMSG_NXTHDR soundness and boundary checks #4903
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
7aeb6e5 to
dd7a629
Compare
af02b51 to
41f8052
Compare
…es in CMSG_NXTHDR musl and its descendants check `next_addr >= max_addr` whilst the rest do `next_addr > max_addr`. This was previously not reflected in the implementations, coming to light only after testing was extended to execute at the controllen boundary. [musl_ref]: https://www.openwall.com/lists/musl/2025/12/27/1
41f8052 to
f624d23
Compare
Contributor
Author
|
@rustbot ready Think that there's only one thread remaining #4903 (comment) I've added the assertion for you to see how it looks in practice. I can then add a test if you think the check is a good idea 😊 (#4903 (comment)) |
tgross35
approved these changes
Jan 4, 2026
Contributor
tgross35
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new wrapping_add bit LGTM, thanks for all the updates!
tgross35
pushed a commit
to tgross35/rust-libc
that referenced
this pull request
Jan 8, 2026
This change makes sure that the header of `next` is within max, returning null if not. This is similar to how `glibc` does it. No checks were previously being done to assert that `next as usize + size_of::<cmsghdr>() < max`. Wrapping offset calculations could then lead to buffer over-reads in the following `(*next).cmsg_len`. [glibc ref](https://github.com/bminor/glibc/blob/b71d59074b98ad4abd23c136ec9ad4c26e29ee6d/sysdeps/unix/sysv/linux/cmsg_nxthdr.c#L49-L51) (backport <rust-lang#4903>) (cherry picked from commit cdc2077)
tgross35
pushed a commit
to tgross35/rust-libc
that referenced
this pull request
Jan 8, 2026
Likely written to make assertions in the unsound CMSG_NXTHDR implementations introduced in rust-lang#1235. CMSG_NXTHDR(mhdr, current_cmsghdr) should not be concerned with the value next_cmsghdr.cmsg_len, which the previous implementation did. (backport <rust-lang#4903>) (cherry picked from commit 1e43edb)
tgross35
pushed a commit
to tgross35/rust-libc
that referenced
this pull request
Jan 8, 2026
Setting `(*pcmsghdr).cmsg_len = cmsg_len as _;` when cmsg_len ranges from 0 to 64 is invalid as it must always be `>= size_of::<cmsghdr>()`, rounded up to the nearest alignment boundary. Some implementations (notably glbic) do check that `cmsg_len >= size_of::<cmsghdr>()` in `CMSG_NXTHDR`, returning null if so. But this is more so an extra precaution that is not mentioned in the POSIX 1003.1-2024. It can therefore not be relied on for tests executed on multiple platforms. The change also removes the ignoring of some testvalues when targeting AIX. (backport <rust-lang#4903>) (cherry picked from commit f391df3)
tgross35
pushed a commit
to tgross35/rust-libc
that referenced
this pull request
Jan 8, 2026
(backport <rust-lang#4903>) (cherry picked from commit 99280f2)
tgross35
pushed a commit
to tgross35/rust-libc
that referenced
this pull request
Jan 8, 2026
(backport <rust-lang#4903>) (cherry picked from commit 2782869)
tgross35
pushed a commit
to tgross35/rust-libc
that referenced
this pull request
Jan 8, 2026
(backport <rust-lang#4903>) (cherry picked from commit befc34b)
tgross35
pushed a commit
to tgross35/rust-libc
that referenced
this pull request
Jan 8, 2026
…es in CMSG_NXTHDR musl and its descendants check `next_addr >= max_addr` whilst the rest do `next_addr > max_addr`. This was previously not reflected in the implementations, coming to light only after testing was extended to execute at the controllen boundary. [musl_ref]: https://www.openwall.com/lists/musl/2025/12/27/1 (backport <rust-lang#4903>) (cherry picked from commit 17adf2d)
Merged
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jan 8, 2026
This change makes sure that the header of `next` is within max, returning null if not. This is similar to how `glibc` does it. No checks were previously being done to assert that `next as usize + size_of::<cmsghdr>() < max`. Wrapping offset calculations could then lead to buffer over-reads in the following `(*next).cmsg_len`. [glibc ref](https://github.com/bminor/glibc/blob/b71d59074b98ad4abd23c136ec9ad4c26e29ee6d/sysdeps/unix/sysv/linux/cmsg_nxthdr.c#L49-L51) (backport <#4903>) (cherry picked from commit cdc2077)
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jan 8, 2026
Setting `(*pcmsghdr).cmsg_len = cmsg_len as _;` when cmsg_len ranges from 0 to 64 is invalid as it must always be `>= size_of::<cmsghdr>()`, rounded up to the nearest alignment boundary. Some implementations (notably glbic) do check that `cmsg_len >= size_of::<cmsghdr>()` in `CMSG_NXTHDR`, returning null if so. But this is more so an extra precaution that is not mentioned in the POSIX 1003.1-2024. It can therefore not be relied on for tests executed on multiple platforms. The change also removes the ignoring of some testvalues when targeting AIX. (backport <#4903>) (cherry picked from commit f391df3)
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jan 8, 2026
…es in CMSG_NXTHDR musl and its descendants check `next_addr >= max_addr` whilst the rest do `next_addr > max_addr`. This was previously not reflected in the implementations, coming to light only after testing was extended to execute at the controllen boundary. [musl_ref]: https://www.openwall.com/lists/musl/2025/12/27/1 (backport <#4903>) (cherry picked from commit 17adf2d)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
O-android
O-arm
O-bsd
O-dragonfly
O-linux
O-linux-like
O-macos
O-mips
O-powerpc
O-redox
O-riscv
O-solarish
O-sparc
O-unix
O-x86
stable-applied
This PR has been cherry-picked to libc's stable release branch
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes:
Properly reflect that musl and some descendants do not allow zero-sized payloads being in
CMSG_NXTHDRmusl_ref.Unsoundness in linux and l4re. Makes sure that the header of next is within max, returning null if not. (Similar to how glibc does it.) It is the fix that should have been made Fix cmsg(3) bugs for musl and OSX #1235, but instead it added pretty severe soundness issues. (Mentioning it for context, not to point fingers.) No checks were previously being done to assert that next as usize + size_of::() < max. Wrapping offset calculations could then lead to buffer over-reads in the following (*next).cmsg_len.
CMSG_NXTHDR Testing: